ci: ensure version is stored when deploying apps #1481
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces several modifications to the CI/CD workflow configurations for production, staging, and YT01 environments, as well as the deployment workflow. Key updates include changes to job conditions for Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
.github/workflows/ci-cd-prod.yml (1)
Line range hint
89-90: LGTM: Well-structured migration controlThe addition of the
runMigrationparameter provides fine-grained control over migration execution while maintaining consistency between dry-run and actual deployment jobs.Consider documenting the migration strategy in the repository's documentation to help future maintainers understand:
- When migrations are triggered
- How to handle manual migration needs
- Rollback procedures for failed migrations
Also applies to: 116-117
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
.github/workflows/ci-cd-prod.yml(1 hunks).github/workflows/ci-cd-staging.yml(1 hunks).github/workflows/ci-cd-yt01.yml(1 hunks).github/workflows/workflow-deploy-apps.yml(1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/workflow-deploy-apps.yml
9-9: property "result" is not defined in object type {outputs: {}}
(expression)
🔇 Additional comments (4)
.github/workflows/ci-cd-staging.yml (1)
99-99: LGTM! The condition change aligns with PR objectives.
The updated condition ensures that the version is stored when apps are deployed, regardless of the migration deployment status. This fixes the issue where version storage was skipped when migration deployment was skipped.
Let's verify that the deployment_executed output is properly defined in the deploy-apps workflow:
✅ Verification successful
The deployment_executed output is properly defined and works as expected
The workflow-deploy-apps.yml file correctly defines the deployment_executed output at the workflow level, which returns true when the deploy-apps job succeeds:
outputs:
deployment_executed:
description: "Indicates if the deployment was actually executed"
value: ${{ jobs.deploy-apps.result == 'success' }}This output is properly referenced in the condition you're reviewing, ensuring version storage occurs when deployments are successful.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the deployment_executed output in workflow-deploy-apps.yml
# Check if the output is defined in the workflow
rg -A 5 'outputs:.*deployment_executed' .github/workflows/workflow-deploy-apps.yml
Length of output: 84
Script:
#!/bin/bash
# Let's check the deploy-apps job definition in the ci-cd-staging.yml file
rg -A 10 'deploy-apps:' .github/workflows/ci-cd-staging.yml
# Also check for any outputs definition in the job
rg -B 5 -A 5 'outputs:.*deployment' .github/workflows/ci-cd-staging.yml
Length of output: 741
Script:
#!/bin/bash
# Let's check the workflow-deploy-apps.yml file for outputs definition
cat .github/workflows/workflow-deploy-apps.yml
Length of output: 12446
.github/workflows/ci-cd-prod.yml (1)
133-133: LGTM: Improved condition for storing app version
The updated condition ensures the version is stored when deployment actually occurs, regardless of migration status, while maintaining proper failure handling. This fixes the issue where skipped migrations would prevent version storage.
Let's verify the deployment_executed output is properly set in the deploy-apps workflow:
✅ Verification successful
Condition change verified and working as intended
The deployment_executed output is properly defined in the workflow-deploy-apps.yml as a reusable workflow output. It's set based on the deploy-apps job's success (jobs.deploy-apps.result == 'success'), which correctly indicates when a deployment has actually occurred. This output is then appropriately used in the condition at line 133 to determine when to store the app version.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify deployment_executed output in workflow-deploy-apps.yml
# Search for the output definition
rg -A 5 'outputs:.*deployment_executed' .github/workflows/workflow-deploy-apps.yml
Length of output: 84
Script:
#!/bin/bash
# Let's first check if the file exists and then search for outputs in the workflow
fd "workflow-deploy-apps.yml" .github/workflows/
# If file not found, let's check the current workflow file for the output
rg -A 5 "outputs:" .github/workflows/ci-cd-prod.yml
# Also search for deployment_executed references
rg "deployment_executed" .github/workflows/
Length of output: 836
Script:
#!/bin/bash
# Let's check the deployment_executed output definition in workflow-deploy-apps.yml
rg -B 5 -A 5 "deployment_executed:" .github/workflows/workflow-deploy-apps.yml
# Also check how it's being set in the workflow
rg -B 3 -A 3 "deployment_executed = " .github/workflows/workflow-deploy-apps.yml
Length of output: 443
.github/workflows/workflow-deploy-apps.yml (2)
Line range hint 134-136: Well-structured job conditions.
The job conditions are well-designed to:
- Run even when migration is skipped (
always()) - Stop on failures (
!failure()) - Stop on cancellations (
!cancelled())
This ensures version storage can proceed while maintaining proper error handling.
🧰 Tools
🪛 actionlint
9-9: property "result" is not defined in object type {outputs: {}}
(expression)
6-9: LGTM: Output implementation aligns with PR objective.
The new deployment_executed output correctly tracks deployment success, enabling downstream workflows to proceed with version storage regardless of migration deployment status.
Let's verify the output usage in downstream workflows:
✅ Verification successful
✅ Output implementation is correctly used in downstream workflows
The deployment_executed output is properly consumed by the store-apps-version job across all environments (YT01, Staging, and Production). The output is used in conditional logic that ensures version storage occurs only when:
- The deployment was actually executed (
deployment_executed == 'true'), or - The workflow was manually triggered (
workflow_dispatch)
This implementation successfully addresses the original issue of skipping version storage when migration deployment is skipped.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if downstream workflows correctly use the new output
# Search for workflows using the deployment_executed output
rg -l "deployment_executed" .github/workflows/
Length of output: 191
Script:
#!/bin/bash
# Check how the deployment_executed output is used in downstream workflows
for file in .github/workflows/ci-cd-{yt01,staging,prod}.yml; do
echo "=== $file ==="
rg -A 3 -B 3 "deployment_executed" "$file"
done
Length of output: 1931
🧰 Tools
🪛 actionlint
9-9: property "result" is not defined in object type {outputs: {}}
(expression)
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
.github/workflows/workflow-deploy-apps.yml (2)
6-9: LGTM! Consider adding description for value expression.The output configuration correctly captures deployment execution status. Consider enhancing the description to explain the conditions that determine when the deployment is considered executed.
outputs: deployment_executed: - description: "Indicates if the deployment was actually executed" + description: "Indicates if the deployment was actually executed (true when both deploy-apps and deploy-jobs succeed and it's not a dry run)" value: ${{ jobs.deploy-apps.result == 'success' && jobs.deploy-jobs.result == 'success' && !inputs.dryRun }}🧰 Tools
🪛 actionlint
9-9: property "result" is not defined in object type {outputs: {}}
(expression)
9-9: property "result" is not defined in object type {outputs: {}}
(expression)
Line range hint
1-1: Consider enhancing error reporting for deployment status.Since we're now tracking deployment execution status, consider adding detailed error reporting to help diagnose deployment failures:
- Add job outputs to capture specific failure points
- Include deployment status in the Slack notification
- Consider implementing deployment rollback on failure
This would improve the observability and reliability of the deployment process.
Would you like me to provide example implementations for these suggestions?
🧰 Tools
🪛 actionlint
9-9: property "result" is not defined in object type {outputs: {}}
(expression)
9-9: property "result" is not defined in object type {outputs: {}}
(expression)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/workflow-deploy-apps.yml(1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/workflow-deploy-apps.yml
9-9: property "result" is not defined in object type {outputs: {}}
(expression)
9-9: property "result" is not defined in object type {outputs: {}}
(expression)
🔇 Additional comments (1)
.github/workflows/workflow-deploy-apps.yml (1)
Line range hint 127-129: LGTM! Job conditions correctly handle migration skip scenario.
The conditions always() && !failure() && !cancelled() on both deploy-apps and deploy-jobs ensure that:
- The deployment continues even if migration is skipped
- The workflow stops on actual failures
- The deployment status is properly captured in the output
This aligns perfectly with the PR objective to prevent skipping the entire workflow when migration is skipped.
Also applies to: 196-198
🧰 Tools
🪛 actionlint
9-9: property "result" is not defined in object type {outputs: {}}
(expression)
9-9: property "result" is not defined in object type {outputs: {}}
(expression)
Description
There is a case where if the migration deployment was skipped, the whole deploy-apps workflow status will also be "skipped". This results in that the store-apps-version will never get run. Rather storing the deployment status excplicitly in the deploy-apps workflow instead.
Related Issue(s)
Verification
Documentation
docs-directory, Altinnpedia or a separate linked PR in altinn-studio-docs., if applicable)Summary by CodeRabbit
New Features
deployment_executedto indicate the success of deployments.Bug Fixes
Documentation